Skip to content

tests: Fix duplicated-path-in-error fail with musl #142301

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

Gelbpunkt
Copy link
Contributor

@Gelbpunkt Gelbpunkt commented Jun 10, 2025

musl's dlopen returns a different error than glibc, which contains the name of the file. This would cause the test to fail, since the filename would appear twice in the output (once in the error from rustc, once in the error message from musl). Split the expected test outputs for the different libc implementations.

Fixes #128474

@rustbot
Copy link
Collaborator

rustbot commented Jun 10, 2025

r? @wesleywiser

rustbot has assigned @wesleywiser.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jun 10, 2025
@rustbot

This comment has been minimized.

@Gelbpunkt Gelbpunkt force-pushed the duplicated-path-in-error-musl branch from 2f57322 to 20d33c5 Compare June 10, 2025 16:41
@@ -1,5 +1,6 @@
//@ only-linux
//@ compile-flags: -Zcodegen-backend=/non-existing-one.so
//@ normalize-stderr: "Error loading shared library .+:" -> "cannot open shared object file:"

// This test ensures that the error of the "not found dylib" doesn't duplicate
// the path of the dylib.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

then on the ERROR down here we will prefix it. the current one will be //[gnu] and the new one will be //[musl], containing the messages we are expecting, so like this:

Suggested change
// the path of the dylib.
// the path of the dylib.
// other stuff I can't comment on because GitHub reviews aren't great
//[gnu]~? ERROR couldn't load codegen backend /non-existing-one.so
//[musl]~? ERROR idk, you're the one making the PR

Copy link
Contributor Author

@Gelbpunkt Gelbpunkt Jun 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, thanks for the hints! I think I got it working as intended now (edit: woops forgot one change, sec)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still hasn't been resolved

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still hasn't been resolved

How exactly? It is resolved AFAICT

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah right, my bad, prefix match 🤦

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a case to be made to make the entire test only-linux && only-gnu since, while it is testing the error output with a nonexistent backend, the description is rather that it tests for the path not to be duplicated, which it is on musl...

Copy link
Member

@jieyouxu jieyouxu Jul 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The original test introduced in #121978 was an anti-regression test for glibc code path's duplicate path error message, so I'm fine with limiting it to gnu-only. But this is fine too.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should I do that instead then or leave it as it is now?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, hm. That original issue duplicated the path in the part that isn't the dlopen error, so I guess it should be fine to keep it as is

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, the current version is fine.

@Gelbpunkt Gelbpunkt force-pushed the duplicated-path-in-error-musl branch from 20d33c5 to 21d3790 Compare June 10, 2025 18:58
musl's dlopen returns a different error than glibc, which contains the
name of the file. This would cause the test to fail, since the filename
would appear twice in the output (once in the error from rustc, once in
the error message from musl). Split the expected test outputs for the
different libc implementations.

Signed-off-by: Jens Reidel <adrian@travitia.xyz>
@Gelbpunkt Gelbpunkt force-pushed the duplicated-path-in-error-musl branch from 21d3790 to 2988b2c Compare June 10, 2025 18:59
@rustbot rustbot added A-compiletest Area: The compiletest test runner A-testsuite Area: The testsuite used to check the correctness of rustc T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) labels Jun 10, 2025
@rustbot
Copy link
Collaborator

rustbot commented Jun 10, 2025

Some changes occurred in src/tools/compiletest

cc @jieyouxu

@Gelbpunkt Gelbpunkt requested a review from workingjubilee June 13, 2025 11:35
@Gelbpunkt
Copy link
Contributor Author

Hi @workingjubilee @wesleywiser, would you mind taking another look at this PR? :)

@fmease fmease added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 11, 2025
@fmease
Copy link
Member

fmease commented Jul 11, 2025

@bors r=workingjubilee,fmease rollup

@bors
Copy link
Collaborator

bors commented Jul 11, 2025

📌 Commit 2988b2c has been approved by workingjubilee,fmease

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 11, 2025
@jieyouxu
Copy link
Member

jieyouxu commented Jul 11, 2025

Sorry, can you please leave a test comment re. why we're revisioning in the first place? It's not super obvious on a cursory glance. Also r=me after comment nit.
@bors r-

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jul 11, 2025
Signed-off-by: Jens Reidel <adrian@travitia.xyz>
@Gelbpunkt
Copy link
Contributor Author

@rustbot ready

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 11, 2025
@jieyouxu
Copy link
Member

@bors r=workingjubilee,fmease,jieyouxu rollup

@bors
Copy link
Collaborator

bors commented Jul 11, 2025

📌 Commit 3ffd47d has been approved by workingjubilee,fmease,jieyouxu

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 11, 2025
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jul 11, 2025
…-musl, r=workingjubilee,fmease,jieyouxu

tests: Fix duplicated-path-in-error fail with musl

musl's dlopen returns a different error than glibc, which contains the name of the file. This would cause the test to fail, since the filename would appear twice in the output (once in the error from rustc, once in the error message from musl). Split the expected test outputs for the different libc implementations.

Fixes rust-lang#128474
bors added a commit that referenced this pull request Jul 11, 2025
Rollup of 10 pull requests

Successful merges:

 - #142301 (tests: Fix duplicated-path-in-error fail with musl)
 - #143403 (Port several trait/coherence-related attributes the new attribute system)
 - #143633 (fix: correct assertion to check for 'noinline' attribute presence before removal)
 - #143647 (Clarify and expand documentation for std::sys_common dependency structure)
 - #143716 (compiler: doc/comment some codegen-for-functions interfaces)
 - #143747 (Add target maintainer information for aarch64-unknown-linux-musl)
 - #143759 (Fix typos in function names in the `target_feature` test)
 - #143767 (Bump `src/tools/x` to Edition 2024 and some cleanups)
 - #143769 (Remove support for SwitchInt edge effects in backward dataflow)
 - #143770 (build-helper: clippy fixes)

r? `@ghost`
`@rustbot` modify labels: rollup
@matthiaskrgr
Copy link
Member

@bors r-
#143790 (comment)

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jul 11, 2025
@Gelbpunkt
Copy link
Contributor Author

@bors r- #143790 (comment)

That's a tricky one. The dlopen error comes from the -gnu host trying to compile for the -musl target. Do we have a way to limit the tests based on the flavor of the host, not the target?

@workingjubilee
Copy link
Member

Huh, odd. I don't believe so.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-compiletest Area: The compiletest test runner A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

tests/ui/codegen/duplicated-path-in-error: Fails on musl libc due to ldso error message difference
8 participants